-
Notifications
You must be signed in to change notification settings - Fork 668
docs: update Vuex guide for namespaced option #691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some feedback, thanks for the contribution!
docs/guides/using-with-vuex.md
Outdated
import Vuex from 'vuex' | ||
import MyComponent from '../../../src/components/MyComponent' | ||
import mymodule from '../../../src/store/mymodule' | ||
import myModule from '../../../src/store/myModule' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this, fits the common convention of camel casing modules
docs/guides/using-with-vuex.md
Outdated
@@ -241,10 +239,11 @@ describe('MyComponent.vue', () => { | |||
|
|||
store = new Vuex.Store({ | |||
modules: { | |||
mymodule: { | |||
myModule: { | |||
namespaced: true, // If store/myModule is namespaced https://vuex.vuejs.org/guide/modules.html#namespacing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessary. I think the examples should have as little noise as possible. Since the user is testing their modules, we can assume they know if they namespaced it or not. Since the focus of this guide and repo is the test setup/assertions, not Vuex and the Vuex API, I don't think this should be included.
Thanks for the PR, I agree with @lmiller1990 Could you also update the example repo to match your changes—https://github.com/eddyerburgh/vue-test-utils-vuex-example |
Updated based on comment, also there is a PR on the mentioned example repo to reflect changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
No description provided.